Make rustdoc JSON Span column 1-based, just like line numbers #139919
Conversation
rustbot
added
A-rustdoc-json
|
rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi Some changes occurred in tests/rustdoc-json |
|
Consider bumping the format version here? Otherwise downstream snapshot tests can't be fixed because the column number is different for different runs within the same format version. |
|
I thought it was done in a follow-up PR. I never remember how it works. ^^' Thanks for the reminder (yep, I completely ignored the bot's message, shame on me). |
|
And done! |
|
No worries at all, teamwork makes the dream work :) |
src/rustdoc-json-types/lib.rs
Outdated
| @@ -30,7 +30,7 @@ pub type FxHashMap<K, V> = HashMap<K, V>; // re-export for use in src/librustdoc | |||
| /// This integer is incremented with every breaking change to the API, | |||
| /// and is returned along with the JSON blob as [`Crate::format_version`]. | |||
| /// Consuming code should assert that this value matches the format version(s) that it supports. | |||
| pub const FORMAT_VERSION: u32 = 43; | |||
| pub const FORMAT_VERSION: u32 = 44; | |||
There was a problem hiding this comment.
After #139393 lands, this should be rebased ontop of master, and then this should be 44->45 instead. See #94591 (comment) for details.
r=me after that.
There was a problem hiding this comment.
Shouldn't we do one release for both and therefore only increase by 1?
There was a problem hiding this comment.
rustdoc-json (the in-tree feature, not the crate) doesn't really do "releases".
Trying to merge them would rely on both PR's first appearing on the same nightly, and that's painful to organize and easy to get wrong. (More of a theoretical concern, but it'd be incorrect if someone was consuming rust from HEAD instead of nightly releases)
There was a problem hiding this comment.
Makes sense. Well, waiting for #139393 to be merged then. :)
aDotInTheVoid
added
S-blocked
GuillaumeGomez
force-pushed
the
rustdoc-json-1-indexed
branch
from
3420ee7 to
076016d
Compare
GuillaumeGomez
removed
the
S-blocked
|
#139393 was merged, I updated the version number to 45. PR is ready for review. :) |
GuillaumeGomez
added
the
S-waiting-on-review
|
Very excited for this to merge so I can avoid needing to support v44 at all :) |
|
@bors r+ |
bors
added
S-waiting-on-bors
…enton Rollup of 8 pull requests Successful merges: - rust-lang#137454 (not lint break with label and unsafe block) - rust-lang#139297 (Deduplicate & clean up Nix shell) - rust-lang#139535 (Implement `Default` for raw pointers) - rust-lang#139919 (Make rustdoc JSON Span column 1-based, just like line numbers) - rust-lang#139922 (fix incorrect type in cstr `to_string_lossy()` docs) - rust-lang#140007 (Disable has_thread_local on i686-win7-windows-msvc) - rust-lang#140016 (std: Use fstatat() on illumos) - rust-lang#140025 (Re-remove `AdtFlags::IS_ANONYMOUS`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#139919 - GuillaumeGomez:rustdoc-json-1-indexed, r=aDotInTheVoid Make rustdoc JSON Span column 1-based, just like line numbers Fixes rust-lang#139906. This PR does two things: 1. It makes column 1-indexed as well, just like lines. 2. It updates documentation about them to mention that they are 1-indexed. I think it's better for coherency to have them both 1-indexed instead of the weird mix we used to have. Docs for `line` and `col` fields can be found [here](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Loc.html#structfield.line). And finally: it adds a regression test to ensure they are indeed 1-indexed. r? `@aDotInTheVoid`
Fixes #139906.
This PR does two things:
I think it's better for coherency to have them both 1-indexed instead of the weird mix we used to have. Docs for
lineandcolfields can be found here.And finally: it adds a regression test to ensure they are indeed 1-indexed.
r? @aDotInTheVoid